Issue/#675 Matchmaking requests from RabbitMQ #788

Askaholic · 2021-05-07T23:42:16Z

I changed a few things from the issue. Mostly I'm using name keys instead of ids for everything except players because that's what the ladder service expects in client messages already.

Probably need to refactor the rating_type on Game objects to allow unrated games by setting rating_type to None.

Closes #675

codecov · 2021-05-07T23:46:39Z

Codecov Report

Attention: 12 lines in your changes are missing coverage. Please review.

Comparison is base (ea2b7d3) 94.61% compared to head (6677516) 96.55%.

❗ Current head 6677516 differs from pull request most recent head d3ed7da. Consider uploading reports for the commit d3ed7da to get more accurate results

Additional details and impacted files

Files	Coverage Δ
server/game_service.py	`99.02% <ø> (+0.55%)`	⬆️
server/gameconnection.py	`95.01% <100.00%> (+3.75%)`	⬆️
server/games/game.py	`95.75% <94.73%> (+2.77%)`	⬆️
server/message_queue_service.py	`96.15% <87.50%> (-1.75%)`	⬇️
server/ladder_service.py	`96.02% <91.66%> (ø)`

... and 62 files with indirect coverage changes

eoinnoble

Much of this is beyond my understanding, but what I do understand seems correct. I think we need to get more of the branches covered, and I have some very minor stylistic suggestions.

server/games/game.py

server/ladder_service.py

eoinnoble · 2021-05-18T21:10:11Z

server/ladder_service.py

+        map_name = request["map_name"]
+        game_name = request["game_name"]
+        participants = request["participants"]


Maybe keep the queue_name logic in one block and pull these out of the request closer to where they're being used.

server/ladder_service.py

eoinnoble · 2021-05-18T21:27:56Z

server/ladder_service.py

+        player_ids = [participant["player_id"] for participant in participants]
+        missing_players = [
+            id for id in player_ids if self.player_service[id] is None
+        ]
+        if missing_players:
+            raise Exception(
+                "players_not_found",
+                *[{"player_id": id} for id in missing_players]
+            )
+
+        all_players = [
+            self.player_service[player_id] for player_id in player_ids
+        ]
+        non_idle_players = [
+            player for player in all_players
+            if player.state != PlayerState.IDLE
+        ]


Certainly not a big deal but feels like we can probably reduce some of this looping?

Suggested change

player_ids = [participant["player_id"] for participant in participants]

missing_players = [

id for id in player_ids if self.player_service[id] is None

]

if missing_players:

raise Exception(

"players_not_found",

*[{"player_id": id} for id in missing_players]

)

all_players = [

self.player_service[player_id] for player_id in player_ids

]

non_idle_players = [

player for player in all_players

if player.state != PlayerState.IDLE

]

all_players = []

missing_players = []

non_idle_players = []

for participant in participants:

player_id = participant["player_id"]

player = self.player_service[player_id]

if self.player_service[player_id]:

all_players.append(player)

if player.state != PlayerState.IDLE:

non_idle_players.append(player)

else:

missing_players.append(player_id)

if missing_players:

raise Exception(

"players_not_found",

*[{"player_id": id} for id in missing_players]

)

eoinnoble · 2021-05-18T21:28:39Z

server/ladder_service.py

+                [
+                    {"player_id": player.id, "state": player.state.name}
+                    for player in all_players
+                ]


Did you want to unpack this list like you did when there were missing_players?

eoinnoble · 2021-05-18T21:30:20Z

server/ladder_service.py

+        queue = self.queues[queue_name] if queue_name else None
+        featured_mod = featured_mod or queue.featured_mod


It would be easier on my poor brain if these two lines were closer in the code to where we're grabbing initial values.

eoinnoble · 2021-05-18T21:31:02Z

server/ladder_service.py

+        host = all_players[0]
+        guests = all_players[1:]


Do we need to check anywhere that len(all_players) >= 2?

Brutus5000 · 2021-11-04T09:46:10Z

Recap of additionally required gw features as discussed in chat:

map name instead of map id (for map generator maps)
access to game settings such as
- full share settings
- maybe unit restrictions
- game args (I want to override player names)
usage of gw as exclusive queue (if you join it you can't join another queue or game)

Askaholic · 2023-12-25T21:14:16Z

This branch now supports:

map name instead of map id (for map generator maps)
access to game settings such as
- full share settings
- maybe unit restrictions
  not supported by the game
- game args (I want to override player names)
  not supported by the game
usage of gw as exclusive queue (if you join it you can't join another queue or game)

The autolobby system already supports setting arbitrary game options, but only for scalar values. Unit restrictions are represented as a nested value under the RestrictedCategories game option key. The lobby server is able to pass this on to the client, but I'm not sure how the client will translate that to a command line argument to the game, and the game doesn't have code to parse a dictionary or list value there anyway.

The player name is not part of the game options, so it would not be able to be set in this way. Supporting that would require starting on the lua side to figure out how that value should be passed in. I would also think this is not a blocker for an initial GW implementation.

Since these rabbitmq requests come from an external service which must be treated as a black box by the lobby server, there is no way for the lobby server to know if a player is in queue for GW or not, so we can't build any 'exclusive queue' logic around it.

Askaholic requested review from cleborys, eoinnoble and Brutus5000 May 7, 2021 23:42

eoinnoble approved these changes May 18, 2021

View reviewed changes

Askaholic force-pushed the issue/#675-matchmaking-from-rabbitmq branch from 6677516 to 4b597cc Compare August 6, 2021 03:07

Askaholic force-pushed the issue/#675-matchmaking-from-rabbitmq branch from 4b597cc to 8c3e205 Compare August 29, 2021 06:59

Askaholic force-pushed the issue/#675-matchmaking-from-rabbitmq branch 2 times, most recently from b2502a0 to e61dc25 Compare October 31, 2021 04:08

Refactor setting map name and map file path to properties

6d97003

Askaholic force-pushed the issue/#675-matchmaking-from-rabbitmq branch 3 times, most recently from 0e463ea to 9fbac70 Compare December 25, 2023 20:49

Askaholic and others added 2 commits December 25, 2023 16:13

Accept matchmaking requests from rabbitmq

ff2e28c

Fix typing

04b0524

Askaholic force-pushed the issue/#675-matchmaking-from-rabbitmq branch from 9fbac70 to 04b0524 Compare December 25, 2023 21:13

Askaholic force-pushed the develop branch 3 times, most recently from 75a18d1 to 0a667d2 Compare December 22, 2024 22:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue/#675 Matchmaking requests from RabbitMQ #788

Issue/#675 Matchmaking requests from RabbitMQ #788

Askaholic commented May 7, 2021

codecov bot commented May 7, 2021 •

edited

Loading

eoinnoble left a comment

eoinnoble May 18, 2021

eoinnoble May 18, 2021

eoinnoble May 18, 2021

eoinnoble May 18, 2021

eoinnoble May 18, 2021

Brutus5000 commented Nov 4, 2021

Askaholic commented Dec 25, 2023 •

edited

Loading

		queue = self.queues[queue_name] if queue_name else None
		featured_mod = featured_mod or queue.featured_mod

Issue/#675 Matchmaking requests from RabbitMQ #788

Are you sure you want to change the base?

Issue/#675 Matchmaking requests from RabbitMQ #788

Conversation

Askaholic commented May 7, 2021

codecov bot commented May 7, 2021 • edited Loading

Codecov Report

eoinnoble left a comment

Choose a reason for hiding this comment

eoinnoble May 18, 2021

Choose a reason for hiding this comment

eoinnoble May 18, 2021

Choose a reason for hiding this comment

eoinnoble May 18, 2021

Choose a reason for hiding this comment

eoinnoble May 18, 2021

Choose a reason for hiding this comment

eoinnoble May 18, 2021

Choose a reason for hiding this comment

Brutus5000 commented Nov 4, 2021

Askaholic commented Dec 25, 2023 • edited Loading

codecov bot commented May 7, 2021 •

edited

Loading

Askaholic commented Dec 25, 2023 •

edited

Loading